Repository navigation
[Server] Build tool schemas with symfony/json-schema - #740
chr-hertel wants to merge 2 commits into
Conversation
32762e3 to
07b0407
Compare
SchemaGenerator describes the parameters of a handler with the component's generateForParameters() instead of parsing type strings itself. It keeps the MCP-specific parts: injectable parameters, reserved parameter names and the #[Schema] attribute.
Input schemas only list the properties a client can send (Direction::Input), and output schemas require the properties that are always present (Direction::Output). A dialect without schema URI makes stripping $schema unnecessary, and definitions keep the plain class name since each schema is a document of its own.
07b0407 to
0a684e5
Compare
| if (isset($parameterLevelSchema['definition']) && \is_array($parameterLevelSchema['definition'])) { | ||
| $mergedSchema = $parameterLevelSchema['definition']; | ||
| foreach ($parameters as $name => $parameter) { | ||
| $property = (array) $schema['properties'][$name]; |
There was a problem hiding this comment.
The component skips callable and resource parameters (isRepresentable() in buildParametersSchema), so the key isn't there. With cb(string $a, ?callable $fn = null) I get Undefined array key "fn" and "fn":{} ends up in the schema, which accepts anything for an argument JSON can't carry. Can we skip the parameter when the component didn't emit it?
| ?JsonSchemaGeneratorInterface $generator = null, | ||
| ?TypeResolverInterface $typeResolver = null, | ||
| private readonly Configuration $configuration = new Configuration(new Dialect('#/$defs/', NullSyntax::Union), definitionName: '', direction: Direction::Input), | ||
| private readonly bool $outputSchemaFromReturnType = false, |
There was a problem hiding this comment.
With a getter and a DateTimeImmutable the schema says {"at": {"type": "string", "format": "date-time"}, "name": ...} (both required) but json_encode() gives {"at": {"date": ..., "timezone": ...}} without name. Since #515 validates structuredContent against outputSchema, turning this on makes these tools fail at call time, and that's the most common class shape. I'd either build structuredContent with a normalizer when the flag is on (that's how we keep Direction::Output true in API Platform) or leave the flag out of 0.11. Also ?Foo gives no schema at all since it's a NullableType, fine but worth a note in the docblock.
| "symfony/deprecation-contracts": "^2.5 || ^3.0", | ||
| "symfony/json-schema": "^8.2@beta", | ||
| "symfony/type-info": "^7.4.6 || ^8.0.6", | ||
| "symfony/uid": "^5.4 || ^6.4 || ^7.3 || ^8.0" |
There was a problem hiding this comment.
property-info ^7.4 makes 5.4/6.4 installs impossible now, can we bump the remaining ^5.4 || ^6.4 ranges (here, finder, http-client, lock, process...) to match what CI tests, and SYMFONY_REQUIRE still defaults to '>=6.4' in the pipeline?
Replaces our schema generator with the new JsonSchema component (
symfony/json-schema8.2.0-BETA1). TypeInfo resolves the parameter types, so the string parsing inSchemaGeneratorcan go (961 => ~190 lines). The second commit is by Nicolas and describes inputs and outputs with their direction.{type: object}_session/_requestnames and#[Schema]outputSchemaFromReturnType) -structuredContentis still built withjson_encode(), which doesn't match what the component describes for getters orDateTimeImmutableminimum-stability: betauntil the component is stablearray<int, V>andarray<array-key, V>are described as a JSON array now, so the object form #643 added for them is gone - that's a BC break in the CHANGELOG, usearray<string, V>for a map. TypeInfo can't tellV[]fromarray<array-key, V>, and lists are what these types mean in practice. I also dropped the tests that only covered the old type-string parser.What we lose: apps that use the Symfony 6.4 Serializer, Validator or PropertyInfo, or pin all Symfony packages to 6.4 via Flex, can't upgrade - the component needs 7.4+ of those. Array shapes are closed now (
additionalProperties: false), so extra keys in a shape argument get-32602- the rest is in the CHANGELOG.@soyuka could you have a look? 🙏